Skip to content

feat(operator): fetch JWT-SVID via go-spiffe SDK, remove spiffe-helper sidecar - #478

Closed
Alan-Cha wants to merge 12 commits into
mainfrom
feat/spiffe-sdk-jwt
Closed

feat(operator): fetch JWT-SVID via go-spiffe SDK, remove spiffe-helper sidecar#478
Alan-Cha wants to merge 12 commits into
mainfrom
feat/spiffe-sdk-jwt

Conversation

@Alan-Cha

Copy link
Copy Markdown
Member

Summary

Closes #477. Replaces the standalone spiffe-helper sidecar container with a direct call to the SPIRE Workload API using the go-spiffe SDK — the same approach authbridge-proxy (the default agent sidecar mode) already uses.

Background

PR #473 added a spiffe-helper sidecar to the operator pod. The sidecar wrote a JWT-SVID to /opt/jwt_svid.token which the operator binary then read from disk. This approach had two problems:

  1. Abandoned image: ghcr.io/kagenti/kagenti-extensions/spiffe-helper is not published by the kagenti-extensions automated pipeline. The standalone image was removed from the pipeline in April 2026 when spiffe-helper was bundled into the combined authbridge image. Only :latest remains from a historical publish.

  2. Inconsistent architecture: authbridge-proxy (the default agent sidecar) already obtains SPIFFE credentials in-process via the go-spiffe WorkloadAPI client — "no spiffe-helper sidecar binary" (Dockerfile comment). The operator using a separate sidecar was inconsistent with this direction.

Changes

clientregistration_controller.go: replace JWTSVIDPath + os.ReadFile with workloadapi.New() + FetchJWTSVID(). Uses the same socket address as verifiedFetch (--verified-fetch-spiffe-socket). Removes path traversal validation (no longer needed).

cmd/main.go: remove --jwt-svid-path flag; SpiffeSocket field on the reconciler reuses verifiedFetchSpiffeSocket.

manager.yaml: remove spiffe-helper sidecar container, jwt-svid emptyDir volume, spiffe-helper-config volume mount. The spiffe-workload-api CSI volume remains (shared with verifiedFetch).

configmap-spiffe-helper.yaml: deleted.

values.yaml: remove jwtSVIDPath value.

Depends on

Checklist

  • All commits signed-off (DCO)
  • Backward compatible — UseSpiffeAuth=false path unchanged
  • Builds and tests pass

Assisted-By: Claude Code

Alan-Cha added 12 commits July 12, 2026 09:30
…nd jwtAudience

Add comments to values.yaml explaining the semantic distinction between the
two flags and what jwtAudience actually represents in the SPIFFE/Keycloak flow.

Signed-off-by: Alan Cha <alan.cha@ibm.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
Keycloak's FederatedJWTClientValidator checks the JWT-SVID aud claim against
the realm's OIDC issuer URL (always the external/public URL). Using the
internal k8s service address causes a silent audience mismatch and
invalid_client_credentials. Document the correct value and how to find it.

Signed-off-by: Alan Cha <alan.cha@ibm.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
…blicUrl

jwtAudience was always going to be keycloak.publicUrl/realms/<realm> —
the issuer URL of the Keycloak realm the operator authenticates to.
There is no meaningful reason to configure it independently.

Remove the user-facing field and derive the audience directly in the
template. keycloak.publicUrl is already required for other reasons (UI,
authbridge) so this does not add a new dependency. Fail at helm install
time with a clear message if keycloak.publicUrl is not set.

Signed-off-by: Alan Cha <alan.cha@ibm.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
v0.5.0-rc.3 is the latest versioned spiffe-helper image tag confirmed to
exist in ghcr.io. Previous tag (v0.6.0-alpha.4) was a GitHub release tag
but the corresponding container image was never published. Update comment
to warn that not all release tags produce container images.

Signed-off-by: Alan Cha <alan.cha@ibm.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
In federated-jwt mode, agents authenticate to Keycloak using their SPIFFE
identity (JWT-SVID) directly via the workload API socket. The operator still
registers the Keycloak client with federated-jwt authenticator type, but no
credential Secret is needed — AuthBridge reads JWT-SVIDs directly, not from
a mounted client-secret file.

Previously the operator wrote a Kubernetes Secret containing the Keycloak-
generated client secret even in federated-jwt mode, even though nothing ever
reads it. This is a hygiene issue (unnecessary secrets in agent namespaces)
and a defense-in-depth gap (secret exists even if Keycloak correctly rejects
it). Skipping creation removes the noise.

Signed-off-by: Alan Cha <alan.cha@ibm.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
Signed-off-by: Alan Cha <alan.cha@ibm.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
… template

The spiffeHelper.image.* values block was introduced to pin the sidecar image,
but ghcr.io/kagenti/kagenti-extensions/spiffe-helper is not published by the
kagenti-extensions automated pipeline and the pinned tag (v0.6.0-alpha.4) does
not exist in the registry. Remove the values block entirely and hardcode :latest
in the template, which is the only published image. A separate issue will track
the correct long-term approach.

Signed-off-by: Alan Cha <alan.cha@ibm.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
…mode

When skipping credential secret creation in federated-jwt mode, also skip
annotating the pod template with the secret name. The webhook mounts the
credential secret based on that annotation — if we skip secret creation but
still set the annotation, pods get stuck in Init because the secret does
not exist.

In federated-jwt mode, AuthBridge reads JWT-SVIDs from the SPIFFE workload
API socket directly and does not need the credential secret at all.

Signed-off-by: Alan Cha <alan.cha@ibm.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
The authbridge webhook pre-populates the Keycloak client-credentials
annotation so the credential Secret volume is ready before the controller
reconciles. In federated-jwt mode, no credential Secret is created —
AuthBridge reads JWT-SVIDs from the SPIFFE workload API socket directly.
Injecting the volume mount for a non-existent Secret left pods stuck in
Init:0/1.

Fix: read CLIENT_AUTH_TYPE from authbridge-config before pre-populating
the annotation, and skip it when federated-jwt is configured.

Signed-off-by: Alan Cha <alan.cha@ibm.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
…efault

Hardcoding :latest in the template was worse — it removed the ability to
override the image via Helm values without forking the chart. Restore the
templated approach with tag: latest as the default, which is the only
reliably-published tag.

Signed-off-by: Alan Cha <alan.cha@ibm.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
Signed-off-by: Alan Cha <alan.cha@ibm.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
…ffe-helper sidecar

The spiffe-helper sidecar approach used an abandoned standalone image from
the kagenti-extensions pipeline (removed April 2026 when spiffe-helper was
bundled into authbridge). The authbridge-proxy itself already uses the
go-spiffe WorkloadAPI client in-process — this change makes the operator
consistent with that architecture.

Changes:
- clientregistration_controller.go: replace JWTSVIDPath + os.ReadFile with
  workloadapi.New() + FetchJWTSVID(). Uses the same socket as verifiedFetch.
  Removes path traversal validation (no longer needed without file I/O).
- cmd/main.go: remove --jwt-svid-path flag; reuse --verified-fetch-spiffe-socket
  for JWT-SVID fetching via SpiffeSocket field on the reconciler.
- manager.yaml: remove spiffe-helper container, jwt-svid emptyDir volume,
  spiffe-helper-config volume mount. spiffe-workload-api CSI volume remains
  (shared with verifiedFetch).
- configmap-spiffe-helper.yaml: deleted (no longer needed).
- values.yaml: remove jwtSVIDPath value.

Signed-off-by: Alan Cha <alan.cha@ibm.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
@Alan-Cha

Copy link
Copy Markdown
Member Author

Closing in favor of clean implementation in #522 which removes spiffe-helper sidecar and uses go-spiffe SDK directly.

@Alan-Cha Alan-Cha closed this Aug 27, 2026
@github-project-automation github-project-automation Bot moved this from New/ToDo to Done in Rossoctl Issue Prioritization Aug 27, 2026
Alan-Cha added a commit that referenced this pull request Aug 28, 2026
* feat: use go-spiffe SDK directly instead of spiffe-helper sidecar

Remove spiffe-helper sidecar from operator pod and use go-spiffe SDK
directly to fetch JWT-SVIDs from the SPIRE Workload API.

Changes:
- operator/cmd/main.go: Remove --jwt-svid-path flag, pass SpiffeSocket
  instead of JWTSVIDPath to controller
- operator/internal/controller/clientregistration_controller.go:
  Replace JWTSVIDPath field with SpiffeSocket, add fetchJWTSVID()
  helper that uses workloadapi.Client.FetchJWTSVID() to get JWT-SVID
- charts/operator/templates/manager/manager.yaml: Remove spiffe-helper
  sidecar container, jwt-svid volume, and --jwt-svid-path CLI arg
- charts/operator/templates/manager/configmap-spiffe-helper.yaml:
  Delete (no longer needed)

This matches the architecture already used by authbridge-proxy and
eliminates the need for maintaining a separate spiffe-helper sidecar.

Closes: #478
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>

* fix: check error from client.Close() in fetchJWTSVID

Address errcheck linter: defer client.Close() now properly checks
and logs any close errors.

Signed-off-by: Alan Cha <Alan.cha1@ibm.com>

* test: add unit tests and documentation for JWT-SVID fetching

Add unit tests for fetchJWTSVID error handling:
- Test invalid socket path
- Test empty socket path
- Document limitations of unit tests vs integration/E2E tests

Add comprehensive testing guide documenting:
- Unit test scope and limitations
- Integration test requirements (future work)
- E2E test procedures with SPIRE
- Manual verification steps for token exchange
- Common issues and troubleshooting

Signed-off-by: Alan Cha <Alan.cha1@ibm.com>

* test: remove documentation page, keep unit tests only

Signed-off-by: Alan Cha <Alan.cha1@ibm.com>

* fix: Use Keycloak token endpoint URL as JWT-SVID audience

JWT-SVID audience must match the token endpoint URL for Keycloak
JWT-SPIFFE authentication, not just the realm name. Keycloak validates
the audience claim and rejects tokens with incorrect audience.

Changes fetchJWTSVID() call to construct the full token endpoint URL:
${KEYCLOAK_URL}/realms/${REALM}/protocol/openid-connect/token

Fixes "Invalid token audience" error during operator authentication.

Assisted-By: Claude Code
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>

* fix: Query Keycloak OIDC discovery to get correct JWT-SVID audience

JWT-SVID audience must match Keycloak's realm issuer URL exactly per RFC 7523.
The issuer is typically a public URL (e.g., keycloak.localtest.me) while the
authbridge-config contains the in-cluster service address.

Changes:
- Add getKeycloakIssuer() to query /.well-known/openid-configuration
- Use issuer URL as JWT-SVID audience instead of service URL
- Add proper error handling and logging for issuer lookup

Fixes "Invalid token audience" error during operator authentication.

Ref: docs/users-guides/authentication.md lines 72-76

Assisted-By: Claude Code
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>

* fix: check error from resp.Body.Close() in getKeycloakIssuer

Fixes errcheck linter error:
  Error return value of `resp.Body.Close` is not checked

Assisted-By: Claude Code
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>

* perf: cache OIDC issuer and reuse workloadapi.Client

Add caching to reduce HTTP and gRPC overhead on frequent reconciles:

- getKeycloakIssuer(): Cache issuer URL by (keycloakURL, realm) using
  sync.Map. Issuer is stable per realm, no need to query OIDC discovery
  endpoint repeatedly.

- fetchJWTSVID(): Reuse long-lived workloadapi.Client instead of
  creating new gRPC connection on every call. Lazily initialize with
  mutex protection.

Addresses suggestions in PR #522 review from cwiklik.

Signed-off-by: Alan Cha <Alan.cha1@ibm.com>

* fix: use double-checked locking and clear stale client on error

Address review feedback from clawgenti:

1. Mutex held during slow gRPC dial: Use double-checked locking to
   release mutex before calling workloadapi.New(). This prevents
   blocking all concurrent reconciles during the connection attempt.

2. Stale client not retried on disconnect: Clear cached client when
   FetchJWTSVID() fails, so next reconcile re-dials instead of
   reusing a dead connection.

Both fixes follow the suggested patterns from PR #522 review.

Signed-off-by: Alan Cha <Alan.cha1@ibm.com>

* fix: address review feedback from clawgenti and mrsabath

1. Use background context for workloadapi.New() to prevent client
   teardown when reconcile context is cancelled. The cached client
   should outlive any single reconcile.

2. Reuse http.Client for OIDC discovery requests (initialized in
   SetupWithManager) instead of allocating fresh on every call.
   Saves a tiny allocation and makes intent clearer.

3. Render --verified-fetch-spiffe-socket flag when operatorAuth is
   enabled, not just verifiedFetch. Uses same OR condition as the
   volume mount. Prevents silent divergence if user customizes
   spiffeEndpointSocket.

Addresses suggestions from PR #522 review.

Signed-off-by: Alan Cha <Alan.cha1@ibm.com>

---------

Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

feat: replace spiffe-helper sidecar with direct go-spiffe SDK call for operator JWT-SVID auth

2 participants